feat(delegate): unsubscribe, subscription introspection, and pin every wire tag by sanity · Pull Request #98 · freenet/freenet-stdlib

sanity · 2026-08-30T20:17:22Z

Problem

Three things, all in the delegate API surface, all of the same shape: something that looks like it is guaranteed and isn't.

1. The wire-format pin covered one variant out of sixteen. inbound_delegate_msg_wire_format_is_stable asserted that InboundDelegateMsg's variant 0 is ApplicationMessage. Nothing covered OutboundDelegateMsg at all, and nothing covered any variant after the first. So any reorder that happened to leave ApplicationMessage in front was undetected — swapping UserResponse and GetContractResponse, for instance, reassigns two bincode tags and makes already-deployed delegate WASM decode each as the other. Silently: the bytes still parse, into the wrong variant.

That is not a hypothetical. Exactly that swap was written and staged during the work that led to this PR, in a change whose stated purpose was to protect the wire format.

2. InboundDelegateMsg's doc comment asserted a fact about the code that was false. It said OutboundDelegateMsg was already #[non_exhaustive]. It never has been. Anyone reasoning about whether a variant addition was source-breaking got the wrong answer from the documentation.

3. A delegate cannot ask what it is subscribed to. A delegate's subscription set lives in the node, not in the delegate: the WASM is instantiated per invocation and dropped immediately after, and the node replays subscriptions across a restart without running the delegate at all. So after a restart a delegate has no way to learn its own state. It can keep a parallel record in its secrets — which drifts from the node's exactly in the cases that matter — or re-subscribe to everything on every wake. freenet/freenet-core#5467 names this as blocking restart-replay.

Approach

Pin every tag, and fail closed in both directions. delegate_msg_variant_tags_are_pinned asserts the bincode tag of every variant of both enums. Two mechanisms keep it honest, because a pin that can rot is not a pin:

The tag map is an exhaustive match. #[non_exhaustive] has no effect inside the defining crate, so adding a variant without pinning it is a compile error.
A probe asserts that the tag one past the last known variant does not decode. Add a variant and update nothing else, and that tag becomes decodable and the test fails. Without this, the variant-count constants would be checked only against a list written by the same hand in the same commit, which is a restatement rather than a check.

PutContractRequest is covered too. The obvious shortcut is to skip it because building a ContractContainer is awkward; it is four lines, and a pin with a hole in it reads as coverage while providing none.

Assert the compatibility rules instead of stating them. delegate_wire_compat and struct_field_wire_compat establish what bincode actually does, because appending an enum variant and appending a struct field break in opposite directions and it is easy to carry the wrong intuition from one to the other:

change	old sender → new receiver	new sender → old receiver
append an enum variant	fine, old tags unchanged	hard error, unknown tag
append a struct field	hard error, unexpected end of input	silently ignored if the struct is terminal in its message; silent corruption if it is not

A struct field is the more dangerous of the two: bincode is positional and carries no field tags, so there is nothing for a decoder to skip. #[serde(default)] does not make a field optional on this path — it is a self-describing-format feature and protects serde_json only, which is easy to misread given ContractState::size_bytes carries it. The practical rule that follows is to prefer a new enum variant over a new field on an existing wire struct, since a variant is only ever seen by a peer that asked for it.

NodeDiagnosticsResponse is terminal in its message, which is the only reason a field can be appended to it without corrupting what follows. That property is invisible at the definition site, so it is now pinned rather than assumed.

Introspection as a host function, not a message variant. DelegateCtx::list_subscriptions is backed by two new V2 host functions in the existing freenet_delegate_contracts namespace. Host functions resolve by name at module instantiation, so this is additive for every existing delegate — one that does not import it is unaffected, and one that does fails to load on a node too old to provide it, with a named missing-import error. Compare a new enum variant, which fails mid-protocol at bincode decode with no way for the delegate to have checked first. Where a capability can be expressed either way, the host function has the better failure mode.

It returns Result<Vec<[u8; 32]>, i64>, not a bare Vec. An empty list and a failed enumeration mean opposite things to a caller deciding whether to re-subscribe, and collapsing them is how a delegate concludes its user's content is unpinned because a host call failed. For the same reason the off-WASM stub returns Err, so a host-side test cannot read "no subscriptions" out of a stub that never had any.

OutboundDelegateMsg stays not #[non_exhaustive], deliberately. The false doc comment is fixed by correcting it, not by making it true. freenet-core dispatches this enum in exhaustive matches with no wildcard (contract.rs, in the request loop and again in the app-message filter). Marking it would force those to grow _ => arms, and a newly added variant would then compile against the host with no handler — the delegate's request silently swallowed while the call reports success. That is the failure mode a delegate SubscribeContractRequest has today (freenet/freenet-core#4669), and the compile error is what stops the next one. InboundDelegateMsg keeps the attribute because its consumers are third-party delegate WASM, which can reasonably ignore an unknown variant. The asymmetry is now documented on both enums, and matches the position already taken in #82.

Also corrects subscribe_contract's doc comment, which said notification delivery was "a follow-up" (it works) while saying nothing about the fact that a delegate subscription registers no demand in the network — it does not pin the contract, enter the renewal set, or exempt it from eviction, so a delegate sees remote updates only while some other route keeps the node subscribed. The call succeeds either way and nothing distinguishes the two, which is why it belongs at the call site rather than in an issue.

Compatibility

Wire change: two variants appended, nothing moved. UnsubscribeContractRequest joins OutboundDelegateMsg and UnsubscribeContractResponse joins InboundDelegateMsg, both at tag 8. Every pre-existing tag is exactly where it was, so deployed delegate WASM built against an older stdlib is unaffected — the append-safe direction of the table above. A hand-built pre-0.9.0 ContractNotification payload is asserted to still decode unchanged.

Tag 8 is a deliberate allocation, coordinated with #82: that PR appends ScheduleWakeup / WakeupFired and moves to tag 9. Recorded as a comment on #82 so it does not live only in a working session.

The pin was made to fail before it was made to pass. Adding the two variants with nothing else changed produced five E0004 non-exhaustive-pattern errors — three in production code, including the FlatBuffers encoder that the OutboundDelegateMsg doc cites as the reason that enum is deliberately not #[non_exhaustive], and two in the tag map itself:

error[E0004]: non-exhaustive patterns: `OutboundDelegateMsg::UnsubscribeContractRequest(_)` not covered
    --> rust/src/client_api/client_events.rs:1543:52   <- production dispatch
    --> rust/src/delegate_interface.rs:1481:15          <- pinned_inbound_tag
    --> rust/src/delegate_interface.rs:1496:15          <- pinned_outbound_tag

So the guard is demonstrated rather than asserted: a variant cannot be appended without both the host-dispatch site and the wire pin refusing to compile.

The two new host imports require a node whose freenet-core registers them. That is a load-time failure by design, not a silent one. The host half is not in this PR — for host functions the usual stdlib-first order is inverted, since core's linker registration references no stdlib type and can land first. Coordinated with the agent owning crates/core/src/contract.rs.

Testing

New: delegate_msg_variant_tags_are_pinned, every_variant_is_covered_by_the_pin, an_unpinned_variant_fails_this_test, an_old_payload_still_decodes_after_appending_a_variant, a_new_variant_does_not_decode_on_an_old_receiver, the five struct_field_wire_compat cases including node_diagnostics_response_is_terminal_in_its_message, and three round-trip cases for the contract-id codec.

The old-payload tests use hand-built byte strings rather than values produced by this crate's own encoder. An encoder-produced payload would only prove the code agrees with itself, which is not the property under test.

[AI-assisted - Claude]

Also in this PR: two fixes in `memory/buf.rs`

Flagged here because it is a file otherwise unrelated to this change, so a reviewer should not meet it by surprise.

StreamingBuffer::from_ptr is a pub unsafe fn that had no documentation, because its entire doc comment — including the # Safety contract — was attached to total_remaining, a safe getter two lines above that takes no pointer. So the getter was documented as "Create a streaming reader from a buffer pointer" with a safety contract about a ptr it does not have, while the constructor whose callers must uphold that invariant said nothing at all. A method had been inserted into the middle of another method's doc block. Splitting them back apart also clears clippy::missing_safety_doc.

The second is a false positive: the non-WASM stub must keep the mangled __frnt__fill_buffer name to match the WASM import it stands in for, so it takes a narrow #[allow(non_snake_case)] with the reason recorded.

Neither is reachable by the lint gate. ci.yml:99 passes no --features. The clippy matrix does cover both wasm32-unknown-unknown and x86_64-unknown-linux-gnu, so target coverage is fine — but default = [], so feature = "contract" is off in both legs, and both of these live behind #[cfg(feature = "contract")]. The contract-side API has therefore never been linted. Filed as #100 rather than fixed here, because CI is a shared Full-tier surface and widening the lint surfaces a backlog of unknown size.

Verification

Run locally, both the gate CI actually applies and a stricter one, because those are different questions — the first is what merges, the second is what stops the next person inheriting warnings:

cargo clippy --target wasm32-unknown-unknown -- -D warnings — clean (CI's exact invocation)
cargo clippy --all-targets --features contract,net,testing,trace -- -D warnings — clean
cargo test --features contract,net,testing,trace — 111 passed, 0 failed; doc-tests 1 passed
cargo build --target wasm32-unknown-unknown --features contract — succeeds, which is the first time anything has compiled the cfg(target_family = "wasm") branch of list_subscriptions

Note for whoever rebases #82

It appends ScheduleWakeup / WakeupFired to these same two enums, at the tag this PR leaves free. Two things to know:

The new pin will catch a colliding append, which is the point — it fails as a compile error, not as a silent renumbering.
feat: scheduled wakeup primitive for delegates (ScheduleWakeup / WakeupFired) #82 currently reverts four fixes merged to main since July, including the fixed_size_field decode-panic hardening and the unknown_union_discriminant change. That is branch staleness rather than intent, but it needs handling in the rebase regardless of which order the two land in.

The pins were verified to run, and to fail

A filtered cargo test that matches nothing prints 0 passed and exits 0. That is indistinguishable from a clean run by exit code, and a reviewer cannot tell the two apart by reading the output either. So every check below asserts a test count, never rc:

wire-compat tests executed:            8   (each named in the output)
struct-field tests executed:           6
list_subscriptions guard tests:       10
rc for a filter matching NOTHING:      0   <- why rc is not evidence

Counting is necessary and not sufficient — a test that runs can still be one that cannot fail. That is exactly what review found in two of these guards, so both were mutated to confirm they now fail when the thing they guard is broken:

Mutation	Result
`INBOUND_VARIANT_COUNT` left stale at 8 after appending a 9th variant	probe fails ✓
terminality fixture reverted to the all-default (all-zero) shape	non-zero-tail assertion fires ✓

Restored tree: 125 passed, 0 failed.

The first mutation is the drift scenario the count constants exist for; the second is the precise regression three reviewers found independently. Neither guard could previously fail for its stated reason.

A note on the external review

codex review converged on the unknown-tag probe defect independently — the strongest signal in the review, since three Claude lenses and a non-Claude model found the same hole from different directions.

Its proposed remedy would never have worked. It recommended asserting ErrorKind::InvalidTagEncoding. bincode never produces that for an enum tag: there is exactly one construction site, in deserialize_option (bincode-1.3.3/src/de/mod.rs:340). An out-of-range variant index goes through idx.into_deserializer() into serde's derived visitor and comes back as ErrorKind::Custom("invalid value: integer ..., expected variant index 0 <= i < N").

The trap is that bincode's description string for that variant reads "tag for enum is not valid" — so the external model was misled by the description exactly as this PR's original code was misled by the name. A test written to its suggestion would have failed permanently while looking correct.

Worth stating as a general point about external review: treat its diagnosis as signal and its remedy as a hypothesis. The diagnosis here was right and valuable; the fix was wrong in a way that would have been easy to apply without checking.

The host half: owner, and why it cannot be linked yet

The unsubscribe variants are useless without a freenet-core handler, and shipping a variant no host handles is the exact defect this workstream exists to remove. So, plainly:

Owner: the agent holding crates/core/src/contract.rs, by the file-ownership rule — contract.rs:915's TODO(#2830) and the V1 outbound dispatch at contract.rs:660-679 are both in that file. Assignment confirmed by the team lead and accepted.
Tracked as: freenet-core#2830, which specified subscribe and unsubscribe together; only subscribe was built.
There is deliberately no PR number here, because one cannot exist yet. freenet-core cannot name UnsubscribeContractRequest until 0.9.0 is published, so the handler is necessarily downstream of this merge and release. Requiring the number as a merge condition would be a deadlock.

The handler will route both teardowns — the ring demand registration and the DELEGATE_SUBSCRIPTIONS notification hook — through a single helper rather than removing them in two places, so the two records cannot drift apart. That is the same desync class as freenet-core#5487, arriving from the opposite direction.

A correction worth recording, because it is the kind of reassurance that quietly replaces a check. An earlier version of this PR said the exhaustive match in freenet-core means a new variant "cannot compile against a host with no handler". That is too strong. It cannot compile without an arm; an arm returning Ok(()) compiles perfectly and is precisely the silent stub in question. This crate proves it — the FlatBuffers encoder has five arms that log the message and drop it. The compile error is a backstop against forgetting, and no backstop at all against stubbing.

Two findings from review that are the same lesson from opposite directions

Recorded because both are cheap to repeat and neither is visible in a passing test run.

A cosmetic edit inverted a guard. Replacing expect_err with unwrap_or_else to make a panic message interpolate turned the assertion into its opposite — unwrap_or_else unwraps Ok and runs the closure on Err, so the test would have demanded the probe succeed. The compiler caught it, but nothing about the change looked like a behavioural one.

Twelve tests existed and had never executed. The test count went 113 → 125 not because tests were added, but because the list_subscriptions guards were extracted out of #[cfg(target_family = "wasm")]. CI runs cargo test on the host only; the wasm32 matrix entries build and lint and execute nothing. The guards were type-checked and unrun — which reads as coverage in the config and provides none. See #100 for the related lint-gate blindness and #101 for four sibling sites with the same unvalidated-length shape.

sanity

Comprehensive PR Review: #98

Summary

PR Title: feat(delegate): subscription introspection, and pin every wire tag
Type: feat (wire-format guards + a new V2 host function)
CI Status: green on the reviewed HEAD; re-running on the fix commit
Linked Issues: freenet-core#5467, #4669, #5487; freenet-stdlib #99, #100
Review tier: Full — touches wire format / protocol serialization on the delegate enums, an always-Full surface
Reviewers run: code-first, testing, skeptical, big-picture (four parallel Claude lenses, each blind to the others), plus codex review as the external non-Claude pass. All five read the checked-out code in an isolated worktree.
Disclosure: I am the author. Every finding below was raised by a reviewer that did not write the code, and I verified each against source before acting on it.

Code-First Analysis

Independent Understanding: four separable things — a new DelegateCtx::list_subscriptions plus two WASM imports and a shared codec; corrections to three doc comments; two test modules pinning bincode variant tags and demonstrating struct-field compat; two unrelated fixes in memory/buf.rs.

Stated Intent: matches, with one exception found by the big-picture lens (below).

Alignment: good on the wire-guard half. The list_subscriptions half was justified by a false premise — now corrected.

Testing Assessment

Coverage Level: adequate after fixes; two guards were vacuous before them.

Test Type	Status	Notes
Unit	✅	112 pass (was 111; off-WASM contract test added during review)
Integration	N/A	library crate; the host half lands in freenet-core
Simulation	N/A	no routing/topology surface
E2E	⚠️	the two new WASM imports cannot be exercised until the core half exists — stated in the PR rather than papered over

Regression Test: present — the shipped ContractState::size_bytes append is pinned as a concrete instance of the rule.

Skeptical Findings

Risk Level: medium before fixes, low after.

The wire-safety claim itself held under adversarial check: both delegate enums are byte-identical to origin/main, no variant or payload field added, removed or reordered, and no generated-file churn rode in. The problems were in the guards and the prose, not the wire.

Concern	Severity	Location	Status
Terminality pin vacuous — all-default fixture encodes as 27 zero bytes, so `ends_with` asserted only "ends in zeros", which survives appending any zero-encoding field	High	`client_events.rs:4126`	Fixed — non-zero fixture, an assertion that the fixture is not all-zero, and a length equality
Unknown-tag probe asserted only `is_err()`, so a variant whose payload rejects zeros fails for the wrong reason and drift goes undetected	High	`delegate_interface.rs:1591`	Fixed — asserts the error names an invalid variant index, plus a control that the last known tag still decodes
`list_subscriptions` could silently under-report: unchecked `i64 as usize` (32-bit on wasm32), `written > len` ignored by `truncate`, and a set that grew between the two calls returning a short list that looks complete	Medium	`delegate_host.rs:743`	Fixed — length validated against a new `MAX_SUBSCRIPTION_LIST_BYTES`, `written > len` rejected, exactly-full buffer re-checked, and the import contract now specifies `ERR_BUFFER_TOO_SMALL`
`#[allow(non_snake_case)]` claimed to suppress nothing	Low	`buf.rs:369`	Rejected — false positive. rustc rejects consecutive interior underscores; I have the clippy output showing it fires

Big Picture Assessment

Goal Alignment: yes for the wire guards. The list_subscriptions justification had drifted and is corrected.

The most valuable finding in the review, and the one I would not have caught: the doc claimed "the node replays subscriptions across a restart without running the delegate at all" — the entire motivation for the API. It is false. DELEGATE_SUBSCRIPTIONS is an in-memory LazyLock<DashMap> with no persistence, so a restart loses delegate subscriptions rather than replaying them. Verified in freenet-core. The API is still worth adding — it is the read side of that capability — but it does not deliver restart-replay until #4669 part 3's durable store lands, and it now says so.

Two further false claims, both verified and corrected:

The #[non_exhaustive] rationale cited SubscribeContractRequest as a variant that compiles with no handler. It is handled (contract.rs:916-940); its defect is that it registers no demand. A different bug with a different fix. The argument stands on its own and now states two honest limits: the compile error forces an arm to exist, not a working handler — this crate's own encoder has arms that log and drop.
The version floor named a stdlib version. Host functions register by name and reference no stdlib type, so that guarantees nothing; corrected to name the core requirement, and to say no released node provides these imports yet.

Anti-Patterns Detected: none of the CI-chasing kind. No test removed, skipped, or loosened; inbound_delegate_msg_wire_format_is_stable is present and byte-identical to main.

Scope Assessment: some creep, acknowledged. Five arguably-separable things. The buf.rs fixes and the CONTRIBUTING line were flagged in the PR body deliberately; the big-picture lens correctly notes that flagging is transparency, not focus. Splitting list_subscriptions out would be defensible.

External Model (codex)

Converged independently on the probe defect (P2, same location), which is the strongest signal in the review — three Claude lenses and the external model found the same hole from different directions.

Its suggested remedy was wrong, instructively. It recommended asserting ErrorKind::InvalidTagEncoding. bincode never produces that for an enum tag: it has exactly one construction site, in deserialize_option (de/mod.rs:340). The trap is that its description string reads "tag for enum is not valid" — codex was misled by the description exactly as the original code was misled by the name. A test written to codex's suggestion would never have passed. The fix asserts the error names an invalid variant index instead, which the passing test empirically confirms.

Documentation

Code docs: complete, and now accurate — four claims corrected against source.
CHANGELOG: corrected. The "additive for every existing delegate" claim is now evidence rather than prose: River's shipped chat_delegate.wasm builds against stdlib 0.8.5, which declares the five freenet_delegate_contracts externs, and wasm-objdump -x shows it imports none of them. (A first attempt to demonstrate this with a synthetic delegate was itself vacuous — the harness exported no entry point, so nothing could import, and an empty module reports zero the same way.)
Noted, not fixed: the repo-root examples/delegate.rs no longer compiles against DelegateInterface — it uses a 5-parameter signature with a SecretsStore argument the trait has not had for some time. It is not wired into any Cargo target, so nothing catches it. Worth its own issue.

Recommendations

Must Fix (Blocking)

All resolved in be638f3:

Terminality pin vacuous.
Unknown-tag probe asserted only is_err().
Restart-replay premise false.
SubscribeContractRequest cited as unhandled.
Version floor named a stdlib version.

Should Fix (Important)

FFI hardening for the truncation and cast paths — done.
Best-effort/lossy notification delivery now documented — done.
"Old delegate → new host always fine" qualified: true for appended variants, not for fields appended to their payload structs — done.

Consider (Suggestions)

Split list_subscriptions into its own PR. Not done — it is the piece the host half is written against, and the ABI is what chokepoint needs.
Pin the request/response enums (HostResponse, QueryResponse, ContractRequest) the same way. Genuinely worth doing and out of scope here; the response direction is entirely unpinned today.
A HIGHEST_TAG_EVER_USED watermark, since removing the last variant is invisible to the whole scheme.
File the stale examples/delegate.rs.

10–12 are follow-ups, not merge blockers.

Verdict

State: Needs Changes — Re-review Required After Fix (fixes are pushed; a re-review pass on the new HEAD is required before merge, per the per-code-content rule — five blocking findings were addressed, which is well past the threshold)

HEAD SHA reviewed: 42ecc979ac9bce4e1ba80e4b27f94baaf55e570d
Fixes pushed as: be638f3

I am not merging this. The review found real defects — including two guards that would have passed while measuring nothing, in a PR whose entire purpose is to stop exactly that — and the code has changed materially since the reviewed SHA.

[AI-assisted - Claude]

Adds `DelegateCtx::list_subscriptions` so a delegate can ask the node what it is subscribed to. Its subscription set lives in the node, not in the delegate: the WASM is instantiated per invocation and dropped afterwards, and the node replays subscriptions across a restart without running the delegate at all, so a delegate had no way to learn its own state (freenet-core#5467). It returns a `Result`, not a bare `Vec` — an empty list and a failed enumeration mean opposite things to a caller deciding whether to re-subscribe. Delivered as a V2 host function rather than a message variant. Host functions resolve by name at instantiation, so this is additive for every existing delegate and fails at load time with a named missing-import error on a node too old to provide it, instead of mid-protocol on a decode. Pins the bincode tag of EVERY variant of both delegate message enums. The previous pin covered `InboundDelegateMsg`'s variant 0 alone, so any reorder that left `ApplicationMessage` first went undetected — including swapping `UserResponse` and `GetContractResponse`, which reassigns two tags and makes deployed delegate WASM read each as the other, silently. That exact swap was written during the work that produced this pin. The guard fails closed both ways: the tag map is an exhaustive match, so a new variant is a compile error until pinned, and a probe asserts the next tag along does not decode. Also asserts the compatibility rules rather than only stating them. Appending an enum variant and appending a struct field break in opposite directions, and a struct field is the more dangerous: bincode is positional with no field tags, so an old payload fails outright on a new receiver, and a new field is skipped cleanly only when the struct is terminal in its message. `#[serde(default)]` does not make a bincode field optional; it protects the serde_json path only. Corrects two false doc comments: `InboundDelegateMsg` claimed `OutboundDelegateMsg` was `#[non_exhaustive]` (it never has been, and it is deliberately staying un-marked so the host cannot gain a variant without a handler), and `subscribe_contract` claimed notification delivery was a follow-up while saying nothing about the fact that a delegate subscription registers no demand in the network (freenet-core#4669). No wire change: no variant is added, removed or reordered. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

…test enums The old-tag-space and envelope types in the compat tests exist to occupy wire space and to be deserialized into, never to be constructed, which trips dead_code under CI's -D warnings. Also drops a doc comment's reference to what a previous version of that same comment said, which is of no use to a reader of the published API. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

…he rule ContractState::size_bytes was appended in #52 (2026-02-18, crate 0.1.36) and ships in every tag from rust-v0.8.0. ContractState is a HashMap VALUE inside NodeDiagnosticsResponse with two more fields after the map, so the appended u64 is not trailing — it shifts what follows, and an older reader does not get a response missing one field, it gets no response at all. Isolates the single variable by using today's String map key, so it measures the appended field rather than the later key change in #70. Scope, stated so the finding is not read as larger than it is: the only external consumer of this query is fdev diagnostics, which ships from core's own tree and is version-matched in practice; River touches NodeDiagnostics only in tests and pins stdlib 0.8.5. The exposure is an fdev built before 2026-02-18 pointed at a newer node. The value here is the rule with a real instance attached, not the instance. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

Its fields are decoded into and never read, which is the test: the decode either fails or produces something wrong. CI denies warnings. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

HostResponse defaults its type parameter to WrappedState, which is what goes over the wire. The terminality pin was instantiating it at Vec<u8>, so it was pinning the layout of a type nobody sends. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

…operty net-wiring's freenet-core#4669 work changes whether a delegate subscribe registers demand, so stating 'registers no demand' as a fact about this API would be wrong the moment their PR merges — the same doc rot in the other direction. Reframed as node behaviour with a tracking reference: pre-#4669 nodes register no demand at all; post-#4669 nodes register it when hosting the contract, and still do not when they can resolve but are not hosting, since a pin on an unheld contract could be neither renewed nor reclaimed. The delegate cannot detect which node it has, and the call reports success in every case. Also documents list_subscriptions' real cost: the node keys delegate subscriptions contract -> delegates, so this is a scan across every contract with any delegate subscription, not O(this delegate's). Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

After freenet-core#4669 a delegate subscription is two records — the notification hook and the demand registration — and they can separate. An eviction that sheds a still-in-use contract clears the demand and leaves the hook, so a list read from the hook alone reports a contract the delegate is no longer pinning. That 'looks subscribed, is not pinned' state is what #5467 exists to make visible, and reproducing it inside the introspection API meant to reveal it would be the same defect one layer up. A delegate replaying this list after a restart would also re-subscribe to things it holds no demand for and believe it had recovered. Promises the narrower meaning deliberately, so tightening the host's answer to the cross-checked set later is a bug fix rather than a breaking change. Reported by net-wiring from the core side, where the divergence is visible and from stdlib it is not. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

Filed by net-wiring, covering both subscription desyncs and why the two obvious fixes are wrong. Points the reader at the mechanism instead of my summary of it, and records that introspection built against the two-record shape wants rewriting when #4669 part 3's single store lands. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

… getter StreamingBuffer::from_ptr is a pub unsafe fn whose entire doc comment, including its '# Safety' section, was attached to total_remaining — a safe getter two lines above that takes no pointer. So the getter was documented as 'Create a streaming reader from a buffer pointer' with a safety contract about a ptr it does not have, and the unsafe constructor, whose callers must uphold that invariant, had no documentation at all. A method had been inserted into the middle of another method's doc block. Splitting them back apart also clears clippy::missing_safety_doc. The other error in this file is a false positive: the non-WASM stub must keep the mangled __frnt__fill_buffer name to match the WASM import it stands in for, so it gets a narrow allow with the reason. Neither is reachable by CI's lint gate, because ci.yml:99 passes no --features: the clippy matrix covers both wasm32 and x86_64, but 'contract' is off in both legs, and both of these live behind #[cfg(feature = "contract")]. Filed as issue #100; not fixed here, because CI is a shared Full-tier surface. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

…fers files A build regenerates rust/src/generated/ with whatever local flatc is present, which is not the one that produced the checked-in files, so any build leaves thousands of lines of unrelated churn in the working tree. Staging explicit paths is what keeps it out; a single 'git add -A' puts a toolchain downgrade into a PR where nobody is looking for one. Found while preparing this branch: the diff against main showed ~4000 lines of generated churn that the committed diff did not contain. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

Multi-lens review found the terminality pin vacuous and three doc claims wrong. Each was verified against source before being changed. Vacuous test, found independently by three reviewers: the terminality pin built NodeDiagnosticsResponse from all defaults, which encodes as 27 ZERO bytes, so ends_with(inner) asserted only that the message ends in zeros. That stays true after appending any field that encodes as zeros, which is exactly the mutation it claims to catch. Now uses a distinctive non-zero fixture, asserts the fixture is not all-zero, and adds a length equality so an appended sibling cannot hide even if its bytes coincide. The unknown-tag probe asserted only is_err(), so a future variant whose payload rejects zeros (a DateTime, a NonZero, a validating deserialize_with) would fail for the wrong reason and leave the count constants drifting undetected. It now asserts the error names an invalid variant index, plus a control that the last known tag still decodes from the same payload, so it cannot go vacuous either way. Wrong claim 1: bincode does NOT reject an unknown enum tag with InvalidTagEncoding. That is produced only for a bad Option discriminant (de/mod.rs:340); deserialize_enum hands the index to serde's derived visitor, producing ErrorKind::Custom naming an invalid variant index. Stated in two doc comments; corrected, and now pinned by the probe above. Wrong claim 2: the node replays subscriptions across a restart is false, and it was the motivating premise for list_subscriptions. DELEGATE_SUBSCRIPTIONS is an in-memory LazyLock DashMap with no persistence, so a restart LOSES them. The API is still right to add, but it is the read side of a capability that needs #4669 part 3's durable store; scoped accordingly. Wrong claim 3: the non_exhaustive rationale cited SubscribeContractRequest as a variant compiled with no handler. It IS handled (contract.rs:916-940); its defect is that it registers no demand. Different bug. The argument stands on its own and now states two honest limits: the compile error forces an arm to exist, not a working handler, and this crate's own encoder has arms that log and drop. Wrong claim 4: the version floor named a stdlib version. Host functions are registered by name and reference no stdlib type, so the stdlib a node was built against guarantees nothing. Says so, and that no released node provides these imports yet. Also hardens the FFI, which reviewers found could silently under-report. Validate that the host length is a multiple of 32 and within a new MAX_SUBSCRIPTION_LIST_BYTES before allocating: usize is 32-bit on wasm32, so an unchecked i64 cast truncates to 0 and surfaces as an empty list, the exact conflation the Result return type exists to prevent. Reject written > len, which truncate would otherwise ignore, leaving zero-filled tail bytes to decode as valid-looking all-zero ids. Re-check on an exactly-full buffer so a set that GREW between the two calls returns ERR_BUFFER_TOO_SMALL rather than a short list that looks complete. The import contract now specifies that requirement, since the host half is written against it. Adds the off-WASM test that list_subscriptions returns Err rather than an empty list; documents that notification delivery is best-effort and lossy; qualifies the old-delegate-to-new-host claim, which holds for appended variants but not for fields appended to their payload structs; and renames a test to what it actually pins. The additive claim is now evidence: River's shipped chat_delegate.wasm builds against stdlib 0.8.5, which declares the five freenet_delegate_contracts externs, and imports none of them. A first attempt to show this with a synthetic delegate was itself vacuous, since that harness exported no entry point, so nothing could import and an empty module reports zero the same way. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

freenet-core#2830 specified subscribe and unsubscribe together; only subscribe was built, and core has carried the TODO(#2830) since. Until now the only way a delegate's subscription was released was the implicit cleanup when the delegate itself was unregistered, so a delegate that had finished with a contract kept holding interest for as long as it existed. Appends UnsubscribeContractRequest to OutboundDelegateMsg and UnsubscribeContractResponse to InboundDelegateMsg, both at tag 8. Every existing tag is unchanged, so deployed delegate WASM is unaffected. Tag 8 is Ian's call, coordinated with freenet-stdlib#82, which appends ScheduleWakeup/WakeupFired and now takes tag 9; recorded as a comment on that PR so the decision does not live only in a working session. Unsubscribing a contract the delegate is not subscribed to reports Ok(()). Not a convenience: the host's teardown already treats an absent client id as a no-op, so an error return would have it inventing a failure it did not have. That reasoning is net-wiring's, from the side that implements it, and it survives someone later deciding convenience was not a good enough justification. The pin was made to fail before it was made to pass. Adding the two variants with nothing else changed produced five compile errors, all E0004 non-exhaustive-pattern: three in production code, including the FlatBuffers encoder in client_api/client_events.rs that the OutboundDelegateMsg doc cites as the reason the enum is deliberately NOT non_exhaustive, and two in the tag pin itself (pinned_inbound_tag, pinned_outbound_tag). So the guard is demonstrated rather than asserted: a variant cannot be appended without both the host dispatch site and the wire pin refusing to compile. Adds a round-trip test for the pair that also asserts a hand-built pre-0.9.0 ContractNotification still decodes unchanged, so the append is shown not to disturb anything older. The host half is freenet-core's and is owned by net-wiring, who has the exact field layout. This does not ship until they confirm they are landing it — shipping a variant no host handles would be the same defect this workstream exists to remove. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

Re-review of the unsubscribe commit found one real design mistake, the oracle problem in my own new test, and a false claim I had just reintroduced. The exactly-full re-check was wrong twice over. It re-called the length function whenever the read came back exactly filling the buffer, meaning to catch a set that had grown. But an exactly-full buffer is the NORMAL result, not an edge case: len is derived from the same set the read serialises. So it doubled a scan the docs describe as O(all contracts with any delegate subscription) on every non-empty call, and it could fail a correct read by reporting ERR_BUFFER_TOO_SMALL when a subscription happened to arrive in between. It was not even sound: grow-then-shrink passes it. Removed. The import contract already requires the host to return ERR_BUFFER_TOO_SMALL rather than truncate, so a short write means the set shrank, and completeness rests on that contract, which is why the contract is stated on the import rather than implied. The decisions now live in validate_list_len and resolve_written, which are pure and compiled on every target. That matters because CI runs cargo test on the host only; the wasm32 matrix entries build and lint but execute nothing, so everything previously inside cfg(target_family = "wasm") was type-checked and never run. Ten table-driven tests now cover the branches, including the wasm32 truncation case (1 << 32 as usize is 0 there, which would have surfaced as an empty list). My round-trip test for the new unsubscribe pair proved only that the code agrees with itself. Both structs' docs say the field ORDER is the wire format, and a round-trip through this crate's own encoder cannot establish that — swapping contract_id and result would round-trip just as happily. Both layouts are now frozen as hand-written bytes, the inbound half asserts the VALUES rather than just the variant, and the Err(String) path is exercised since it has a different bincode shape from Ok. Reintroduced false claim, the same defect class as this PR's headline fix: the doc said "several of them are #[non_exhaustive]" of the payload structs. Exactly one is, ApplicationMessage. Corrected and named. Also: the #82 note asserted that PR takes tag 9, which it does not yet — it still declares 8, so the text now says it must move and that the pin will catch whichever lands second. The unknown-tag probe gained an outbound control to match the inbound one. The terminality fixture's non-zero guard asserted some byte was non-zero when what ends_with relies on is a non-zero TAIL. Fixes a pre-existing bug found in review: get_context and get_mut_context returned None for UserResponse, which carries a context, because a `_ => None` wildcard swallowed the missing arm and nothing in the crate called either accessor. Arm added, both accessors are now exhaustive with no wildcard, and a table-driven test drives them off every_inbound/every_outbound so the next omission is a compile error rather than a silent None. Filed #101 for four sibling sites with the same unvalidated-length shape. One of these fixes was itself wrong first: replacing expect_err with unwrap_or_else to make a panic message interpolate inverted the test, since unwrap_or_else unwraps Ok and runs the closure on Err. The compiler caught it. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

0.9.0 is already published (crates.io, 2026-09-04) and `main` sits at it, so this change cannot ship under the version it was written against. Appending `UnsubscribeContractRequest` to `OutboundDelegateMsg`, which is deliberately not `#[non_exhaustive]`, is a source-breaking change for every downstream exhaustive match. On a 0.x line that is a minor bump, so 0.10.0 rather than 0.9.1. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV

…racking issue Re-review findings on the rebased head. - Five comments said the unsubscribe pair was "appended in 0.9.0". This PR is what moves the crate off 0.9.0, which is already published, so the pair ships in 0.10.0. Anyone asking "which stdlib do I need for unsubscribe" got the wrong answer from five separate places, in a PR whose purpose is precision about wire versioning. The 0.9.0 reference at the RegisterDelegateWithPredecessors removal is genuine history and is deliberately left alone. - The doc claimed the FlatBuffers encoder has "five" log-and-drop arms. It has six -- this PR's own UnsubscribeContractRequest arm is one of them, so the count was stale the moment it was written. - freenet-core#2830 was cited as the tracking issue for the host-side unsubscribe handler. It is CLOSED as COMPLETED: it specified subscribe and unsubscribe, only subscribe was built, and it was closed anyway. Filed freenet-core#5600 for the outstanding half. Also investigated and dismissed a reported intermittent test failure under `--features contract`. It did not reproduce: 140 consecutive runs clean across both feature sets, including under CPU load, in a worktree with no other process in it. The original observation came from a worktree that two reviewers were sharing while one of them was appending and recompiling probe modules, which is sufficient to explain it. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV

sanity

Re-review after rebase — Full tier

Required because the content changed: rebased onto main (029a31a), re-versioned, and the prior review's verdict was explicitly "Needs Changes — Re-review Required" with five blocking fixes unreviewed.

Reviewed HEAD: 46bdd33 · Fixes pushed as: 4c33b95
Lenses: code-first, skeptical, testing, big-picture (four parallel Claude reviewers, each blind to the others) plus codex review as the external non-Claude pass. Five total.
Tier: Full — wire format / protocol serialization, an always-Full surface.
Disclosure: I rebased and re-versioned this PR but did not write its substance. Every finding below came from a reviewer that did not do that work, and I verified each against source before acting.

The rebase

Only CHANGELOG.md conflicted; every code file applied clean across all 13 commits. The resolution kept both sides' additions, with one judgement call worth naming:

A semantic conflict git could not see. This branch carried a ### TypeScript SDK — Breaking (next release must be 0.4.0, not a patch) section. main has since done that release (55dcc2d), so it now carries ### TypeScript SDK 0.4.0 — Breaking with the same body. Git auto-merged the shared body and left both headers, producing a changelog that announced a release as forthcoming directly above the entry recording it as done. Dropped the stale forward-looking header, kept main's released one.

Confirmed the rebase preserved intent: diff vs main is +1818/-23 across the same 8 files, and no generated FlatBuffers file was touched — worth checking explicitly because main gained a bindings-drift CI job (#110, #125) after this branch was cut. That job passes.

Findings

#	Finding	Severity	Status
1	Five comments said the unsubscribe pair was "appended in 0.9.0"	Medium	Fixed
2	Doc claimed the FlatBuffers encoder has "five" log-and-drop arms; it has six	Low	Fixed
3	freenet-core#2830, cited as the tracking issue for the host half, is CLOSED as COMPLETED	Low	Fixed — filed freenet-core#5600
4	Intermittent test failure / suspected memory corruption under `--features contract`	High (claimed)	Not reproducible — dismissed, see below

(1) is the one that would have cost someone real time. 0.9.0 is already published and this PR is what moves off it, so the pair ships in 0.10.0. Five separate comments gave the wrong answer to "which stdlib do I need for unsubscribe", in a PR whose entire purpose is precision about wire versioning. The 0.9.0 reference at the RegisterDelegateWithPredecessors removal is genuine history and was deliberately left alone.

(2) was stale the moment it was written — this PR's own UnsubscribeContractRequest arm is the sixth.

(3) #2830 specified subscribe and unsubscribe, only subscribe was built, and it was closed as completed anyway. The TODO(#2830) at contract.rs:933 is accurate about the code but points at an issue that reads as resolved. Filed freenet-core#5600 for the outstanding half rather than reopening, so #2830 keeps its accurate history.

On finding (4), because dismissing a High needs evidence

The code-first lens reported two failures in ~37 runs — corrupted bytes in a wire test, and a wrong result from resolve_written(320, 352), a pure function with no I/O — and reasonably read the combination as UB in the raw-pointer paths.

It does not reproduce. 140 consecutive clean runs, across both --features contract and --features contract,net,testing,trace, including under saturating CPU load, in a worktree with no other process in it.

The explanation is mine to own: I gave all four reviewers one shared worktree. The testing lens independently reported seeing a zzz_probe module it did not write being appended to delegate_interface.rs by another process, with a live rustc running — i.e. one reviewer was mutating and recompiling the sources while another executed the test binary. That is sufficient to produce exactly what was observed, and it is the failure mode never-trust-cwd-with-parallel-agents exists to prevent. The finding was a real observation of a real artifact; the artifact was my process error, not this code.

Recorded rather than quietly dropped, because "could not reproduce" is the phrase under which genuine intermittent faults get buried.

What the lenses verified rather than accepted

The prior review found four doc comments asserting false facts, so claims were checked against source:

DELEGATE_SUBSCRIPTIONS is in-memory only — true (native_api.rs:41-43), no persistence path anywhere it is touched. A restart loses delegate subscriptions.
freenet-core matches OutboundDelegateMsg exhaustively with no wildcard — true at four sites (contract.rs:677, contract.rs:2187, execution.rs:365, execution.rs:62).
No released node registers the list_subscriptions imports — true; core registers five functions in freenet_delegate_contracts (wasmtime_engine.rs:2048-2113), not these two.

No fifth false claim was found.

Guards were made to fail

The testing lens applied real mutations and confirmed each guard fires:

Mutation	Result
Swapped `UserResponse`/`GetContractResponse` declaration order	tag pin fails (`left: 2, right: 1`)
Swapped field order in `UnsubscribeContractResponse`	hand-built byte comparison fails
Added a trailing field after `NodeDiagnosticsResponse`	terminality pin fails
Removed `decode_contract_id_list`'s `remainder().is_empty()` check	truncation test fails
Removed `resolve_written`'s `written > len` guard	fails (`Ok(352)` for expected `Err(-8)`)

It also confirmed the prior review's "twelve tests never executed" defect is genuinely fixed: zero new tests sit behind #[cfg(target_family = "wasm")], and all 32 execute under CI's exact invocation.

External model

codex review --base origin/main: "No actionable correctness defects were found. The new wire variants, context accessors, and subscription-list validation are internally consistent." It independently swept the version references and reached the same set I had corrected.

Non-blocking follow-ups

examples/delegate.rs is stale and wired into no Cargo target — its process() has a 5-arg signature the trait has not had for some time. Pre-existing; worth an issue.
The "exactly-full buffer is fine" assumption in resolve_written rests on a freenet-core contract that does not exist yet. Whoever implements the host half should treat "wrote exactly out_len" as ambiguous and return ERR_BUFFER_TOO_SMALL on any race rather than truncating silently. Noted on freenet-core#5600.

Verdict

Approve-equivalent, subject to CI green on 4c33b95. All four findings are fixed or specifically dismissed with evidence. I am not merging it — that decision is the team lead's and Ian's, and freenet-stdlib#82 is stacked on this branch and should land in the same sequence.

[AI-assisted - Claude]

sanity · 2026-09-07T20:46:38Z

Merged with an admin bypass, recorded here so the audit trail explains itself.

Why a bypass was needed, and why it is not a skipped gate. main's classic branch protection has required_pull_request_reviews enabled with required_approving_review_count: 0 — a configuration that blocks every merge while requiring nothing that can satisfy it. The Freenet Developers team holds a bypass_pull_request_allowances, which is how #125, #110 and #105 all merged with zero reviews. Ian authorised the bypass for this merge specifically.

The actual quality gate was passed, and it is stricter than the branch policy: a four-lens Full-tier re-review plus an external codex pass on the rebased content, with three real findings fixed — five comments claiming variants were "appended in 0.9.0" (this PR is what moves off it), a doc miscounting the log-and-drop encoder arms, and the discovery that freenet-core#2830, cited here as the tracking issue, was closed as COMPLETED having specified subscribe and unsubscribe when only subscribe was built (now freenet-core#5600). CI green on 4c33b95, all eight required contexts, 0 behind main.

The rebase was clean in a way worth recording, since the branch carried a warning that it "reverts four fixes": only CHANGELOG.md conflicted, all 13 commits replayed with every code file clean, and the diff against main was unchanged at +1818/-23. That warning was true of a merge and false of a rebase — the same thing was true of #82. There was one conflict git could not see: the branch carried a "next release must be 0.4.0" header while main had since done that release, so the merged result announced a release directly above the entry recording it as done. The stale header was dropped.

One finding from this PR's own machinery, worth keeping: the variant-tag pin added here got its first real workout when #82 was rebased onto it — four E0004 errors plus a failing probe forced the retag from 8 to 9. It worked exactly as designed, which is not something one usually gets to observe about a pin.

Next: #82 retargets to main automatically now, which is the point at which it gets CI for the first time — ci.yml only triggers on PRs targeting main, so while it was stacked here it showed one green check and no red ones, which reads as passing and was not. That trap is now documented in docs/wire-format.md (#129).

[AI-assisted - Claude]

`DelegateCtx::subscribe_contract`'s doc now describes at length that a subscribe may register no demand, and that a delegate cannot detect which it got. This adds the way to detect it. `true` means "the node accepted the registration", not "the contract is pinned". A registration accepted without durable demand is reported identically to one that pinned, so the delegate believes it holds durable interest and does not: the contract stays evictable and notifications later stop at a moment the delegate never observes. Adds `subscribe_contract_checked`, returning `SubscribeOutcome` (`Pinned` / `NotPinned` / `Unrecognized`). `is_pinned()` is true only for `Pinned`: an outcome this build cannot interpret is not evidence of a pin, and folding it into success would reintroduce the exact defect. This is deliberately NOT a wire-format change. It is a host function and a plain Rust type, so it takes no bincode variant tag and cannot shift one -- additive in both directions. A delegate that does not import it is unaffected; one that does fails to *instantiate* on too old a node with a named missing-import error, loudly and at load time. Same reasoning as `list_subscriptions`, which this complements rather than duplicates: that reads the subscription set, which contains accepted-but-unpinned registrations, so it answers "what did I register?" where this answers "what actually got pinned?". `subscribe_contract` is left behaviourally unchanged; altering its return would change the behaviour of already-deployed delegate WASM. Reapplied onto main after #98 merged rather than rebased: #98 was squash-merged, so replaying this branch would have re-applied #98's own commits against a main that already contains them. Reapplying also let this build on #98's much richer subscribe_contract doc, pointing at the new method instead of overwriting what that doc now explains. No version change -- main is already at 0.10.0 from #98. Host half: freenet/freenet-core#5565. No released node provides the import yet. Verified: 130 tests pass; the unknown-outcome guard was mutated (unknown folded into `Pinned`) and confirmed to FAIL. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV

Two findings from the wire review, both free to fix now. `SubscribeOutcome` was missing from `prelude`'s explicit re-export list, so `use freenet_stdlib::prelude::*` brought in `subscribe_contract_checked` but not the type its result must be matched on. Matching the outcome is the entire point of the API, and it needed a second, differently shaped import to do it. The error type is now `i64`, matching both the host function's own return type and `list_subscriptions` from #98. The previous `i32` narrowed with `unwrap_or(i32::MIN)`, so a code outside i32 range became -2147483648 -- a value no host ever sent and that appears in no error table. Every available narrowing answer invents a code; not narrowing does not. The prelude guard was rewritten after the first version proved vacuous. It used `use crate::prelude::*` inside a test module that already has `use super::*` in scope, so the name resolved from the parent module and the test passed with the re-export deleted -- confirmed by deleting it. It now names `crate::prelude::SubscribeOutcome` in full, and deleting the re-export fails the build with E0433. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV

…ounds Round-2 findings. M1 is behavioural; the rest are the same defect in prose, which is that this crate was describing obligations on an unwritten host in the present tense. M1: `schedule_wakeup` now clamps `after` up to MIN_WAKEUP_DELAY itself. The doc said the host "will honour" the floor, that the constant "states the contract so a delegate author can rely on it", and that "#3972 enforces it" -- three lines above a paragraph correctly saying no host does anything yet. Two registers in one comment, and the second was the honest one. One line in the wrapper makes the sentence true of the API the delegate actually calls, and it is how MAX_WAKEUP_TAG_BYTES was already handled; enforcing one documented bound locally while delegating the other to an unwritten host had no stated reason. The clamp is a pure `clamp_wakeup_delay` rather than an inline `.max()`, because the call site is inside `cfg(target_family = "wasm")` and CI never executes that branch -- the wasm32 jobs build and lint only. A clamp living only there would be type-checked and unverified, which is the "twelve tests that never ran" shape #98 found in this same crate. M3: the test comment said the tag cap "must be enforced before the host call, not by the host". That is backwards, and it was addressed to the person who has to write the host check. `__frnt__delegate__schedule_wakeup` is an ordinary WASM import, so a delegate can declare its own extern block and pass a 10 MB tag without ever touching DelegateCtx. A guest-side check on a guest-controlled import is fail-fast convenience and can never be a bound. Only the host can enforce it. Generalised in the comment, since it is true of every bound this crate documents. M2: "the host bounds how many wakeups a delegate may hold pending" was present tense in two places and false. It is load-bearing rather than incidental -- the reader is told a count cap exists in order to explain why a size cap is also needed -- so it is now an obligation on #3972 alongside durability. M4/L1: deleted a stale CHANGELOG run. It claimed `Duration` was on the wire with a byte layout pinned by a test; neither survived the M1 conversion, which moved the delay to `after_millis: i64` across the FFI and removed the outbound pin entirely. It also carried a duplicate durability section and a `### Why a delay` heading that escaped the entry it belonged to. Replaced with a Compatibility section describing what actually ships. L2: removed the superseded no-context rationale from the `get_context` comment. Two rationales coexisted and the weaker one was what a maintainer hits first when editing the accessor. Verified: 133 tests pass; both clippy invocations clean; fmt clean. The clamp guard was mutated to an identity and confirmed to FAIL. Turning `<` into `<=` is an equivalent mutant -- identical output for every input -- so it is not evidence of a gap either way. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV

…ct (#128) * feat(delegate): report whether a subscribe actually pinned the contract `DelegateCtx::subscribe_contract`'s doc now describes at length that a subscribe may register no demand, and that a delegate cannot detect which it got. This adds the way to detect it. `true` means "the node accepted the registration", not "the contract is pinned". A registration accepted without durable demand is reported identically to one that pinned, so the delegate believes it holds durable interest and does not: the contract stays evictable and notifications later stop at a moment the delegate never observes. Adds `subscribe_contract_checked`, returning `SubscribeOutcome` (`Pinned` / `NotPinned` / `Unrecognized`). `is_pinned()` is true only for `Pinned`: an outcome this build cannot interpret is not evidence of a pin, and folding it into success would reintroduce the exact defect. This is deliberately NOT a wire-format change. It is a host function and a plain Rust type, so it takes no bincode variant tag and cannot shift one -- additive in both directions. A delegate that does not import it is unaffected; one that does fails to *instantiate* on too old a node with a named missing-import error, loudly and at load time. Same reasoning as `list_subscriptions`, which this complements rather than duplicates: that reads the subscription set, which contains accepted-but-unpinned registrations, so it answers "what did I register?" where this answers "what actually got pinned?". `subscribe_contract` is left behaviourally unchanged; altering its return would change the behaviour of already-deployed delegate WASM. Reapplied onto main after #98 merged rather than rebased: #98 was squash-merged, so replaying this branch would have re-applied #98's own commits against a main that already contains them. Reapplying also let this build on #98's much richer subscribe_contract doc, pointing at the new method instead of overwriting what that doc now explains. No version change -- main is already at 0.10.0 from #98. Host half: freenet/freenet-core#5565. No released node provides the import yet. Verified: 130 tests pass; the unknown-outcome guard was mutated (unknown folded into `Pinned`) and confirmed to FAIL. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV * fix(review): export SubscribeOutcome from the prelude, and return i64 Two findings from the wire review, both free to fix now. `SubscribeOutcome` was missing from `prelude`'s explicit re-export list, so `use freenet_stdlib::prelude::*` brought in `subscribe_contract_checked` but not the type its result must be matched on. Matching the outcome is the entire point of the API, and it needed a second, differently shaped import to do it. The error type is now `i64`, matching both the host function's own return type and `list_subscriptions` from #98. The previous `i32` narrowed with `unwrap_or(i32::MIN)`, so a code outside i32 range became -2147483648 -- a value no host ever sent and that appears in no error table. Every available narrowing answer invents a code; not narrowing does not. The prelude guard was rewritten after the first version proved vacuous. It used `use crate::prelude::*` inside a test module that already has `use super::*` in scope, so the name resolved from the parent module and the test passed with the re-export deleted -- confirmed by deleting it. It now names `crate::prelude::SubscribeOutcome` in full, and deleting the re-export fails the build with E0433. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV * fix(review): define Pinned against what core actually provides BLOCKER from the wire review. `Pinned` said the contract "enters the renewal set" and is "exempt from eviction". Neither has a referent, and this matters because CODE_PINNED = 0 is about to freeze as host ABI. Verified against freenet-core: `DELEGATE_SUBSCRIPTIONS` appears nowhere in `ring/hosting` -- it is inserted on subscribe, read for notification fan-out, cleaned up on unregister, and feeds no hosting decision. And eviction exemption is not on offer to anyone: `demand-driven-hosting.md` names three demand sources and never mentions delegates, says of the subscriber dimensions that "neither is an absolute pin", and `ring/hosting/cache.rs:899` says a subscribed contract is "NOT hard-pinned". Piece E is actively removing the durable-regardless-of- demand copy as an anti-pattern. So this was not an unimplemented feature; it promised the one thing the architecture is moving away from. The consequence was worse than a wrong doc: a correct host would return NotPinned unconditionally, making the API a constant, and a delegate following the advice would back off forever against a condition that never clears -- inverting the purpose, since it exists so a seller learns their 51st order is not live. `Pinned` now means what the PR's own implementer note already said: the node holds state for this contract and has recorded the delegate's interest. Observable today, needs no hosting-policy change, and is honest about its scope -- it states that no subscription of any kind is an absolute pin, so this is a claim about now rather than durability. `NotPinned` is correspondingly retryable and clears once the node holds the state, which is the ordinary startup case. Deliberately did NOT add CODE_CAP_EXCEEDED: `#[non_exhaustive]` plus `Unrecognized` means a later addition degrades safely, and once Pinned means "holds state" the cap distinction stops carrying its own weight. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV * fix(review): a delegate subscription does not affect eviction ordering M5. The `Pinned` doc said "no subscription of any kind is an absolute pin -- a subscribed contract is ordered last for eviction, not exempt from it". The first half is right. The trailing clause is true of CLIENT subscriptions and false of delegate ones: DELEGATE_SUBSCRIPTIONS feeds nothing in ring/hosting, so a delegate subscribe contributes no hosting demand and does not move the eviction order at all. In a delegate-facing doc the reader applies that sentence to their own subscription, so it read as a weak guarantee where there is none. Now says what is true: it registers notification interest only. Also adds the cross-PR sentence. This check happens at subscribe time and nowhere else, so a subscription that was live can quietly stop being so; re-checking without a UI needs freenet-stdlib#82's schedule_wakeup. Neither change alone closes freenet-core#5565's scenario -- this one lets a delegate learn the truth, that one lets it ask again -- and an adopter reading `Pinned` as fire-and-forget would get exactly the silent failure the PR exists to remove. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV * docs: scope the no-eviction-demand claim to pre-#4669 core Comment-only. `Pinned` said a delegate subscription "does not affect eviction ordering at all", unqualified, in the same file as a doc saying that changes once #4669 lands -- and freenet-core#5493 implements #4669 and is open right now, so this is not a distant hypothetical. True today and defensible, since `Pinned` is already scoped as a statement about now. But an unqualified "at all" is the kind of sentence a reader carries away without its scope, so it now names the scope inline. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV

…ounds Round-2 findings. M1 is behavioural; the rest are the same defect in prose, which is that this crate was describing obligations on an unwritten host in the present tense. M1: `schedule_wakeup` now clamps `after` up to MIN_WAKEUP_DELAY itself. The doc said the host "will honour" the floor, that the constant "states the contract so a delegate author can rely on it", and that "#3972 enforces it" -- three lines above a paragraph correctly saying no host does anything yet. Two registers in one comment, and the second was the honest one. One line in the wrapper makes the sentence true of the API the delegate actually calls, and it is how MAX_WAKEUP_TAG_BYTES was already handled; enforcing one documented bound locally while delegating the other to an unwritten host had no stated reason. The clamp is a pure `clamp_wakeup_delay` rather than an inline `.max()`, because the call site is inside `cfg(target_family = "wasm")` and CI never executes that branch -- the wasm32 jobs build and lint only. A clamp living only there would be type-checked and unverified, which is the "twelve tests that never ran" shape #98 found in this same crate. M3: the test comment said the tag cap "must be enforced before the host call, not by the host". That is backwards, and it was addressed to the person who has to write the host check. `__frnt__delegate__schedule_wakeup` is an ordinary WASM import, so a delegate can declare its own extern block and pass a 10 MB tag without ever touching DelegateCtx. A guest-side check on a guest-controlled import is fail-fast convenience and can never be a bound. Only the host can enforce it. Generalised in the comment, since it is true of every bound this crate documents. M2: "the host bounds how many wakeups a delegate may hold pending" was present tense in two places and false. It is load-bearing rather than incidental -- the reader is told a count cap exists in order to explain why a size cap is also needed -- so it is now an obligation on #3972 alongside durability. M4/L1: deleted a stale CHANGELOG run. It claimed `Duration` was on the wire with a byte layout pinned by a test; neither survived the M1 conversion, which moved the delay to `after_millis: i64` across the FFI and removed the outbound pin entirely. It also carried a duplicate durability section and a `### Why a delay` heading that escaped the entry it belonged to. Replaced with a Compatibility section describing what actually ships. L2: removed the superseded no-context rationale from the `get_context` comment. Two rationales coexisted and the weaker one was what a maintainer hits first when editing the accessor. Verified: 133 tests pass; both clippy invocations clean; fmt clean. The clamp guard was mutated to an identity and confirmed to FAIL. Turning `<` into `<=` is an equivalent mutant -- identical output for every input -- so it is not evidence of a gap either way. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV

…upFired) (#82) * feat: scheduled wakeup primitive for delegates (ScheduleWakeup / WakeupFired) Adds the two wire variants the host<->delegate protocol needs for a delegate to run periodic background work with no UI attached: OutboundDelegateMsg::ScheduleWakeup { after: Duration, tag: Vec<u8> } InboundDelegateMsg::WakeupFired { tag: Vec<u8> } Both appended at tag 9, behind the unsubscribe pair at tag 8. Driving use case is River's weekly secret rotation (freenet/river#228), which today has to live in a client sync loop that stops when the tab closes. `after` is a DELAY, not a deadline, and that is the load-bearing decision. An earlier draft used `at: SystemTime`; a delegate cannot fill that field. Delegates compile to wasm32-unknown-unknown, where `SystemTime::now()` compiles and then panics at runtime, and freenet-core registers no temporal host function in any of the four delegate namespaces -- so there is no value a delegate could compute for an absolute deadline, including the "one week from now" this primitive exists to express. A delay is also strictly more capable. If a clock host function is ever added, absolute scheduling is `target - now` in terms of this same field; an absolute field would gain nothing it did not already need that clock for. A delegate re-arming inside its handler just asks for the same delay again, so the recurring case needs neither a clock nor a timestamp on the fire -- which avoids adding a field the host could only fill meaningfully or leave permanently useless. It also removes two failure modes rather than an ambiguity: a pre-epoch `SystemTime` fails to serialize on the SENDER, and wall-clock steps are now the host scheduler's business rather than a semantic left undefined on the wire. `Duration` encodes identically (u64 secs LE + u32 nanos), so the wire size is unchanged. Durability is stated as a REQUIREMENT ON THE HOST and explicitly not implemented. A week-long delay is only useful if pending wakeups survive a restart, no host does that today, and nothing in this crate can make it true. The precedent runs the wrong way: DELEGATE_SUBSCRIPTIONS, the one comparable piece of per-delegate host state, is an in-memory LazyLock<DashMap> a restart discards. A wire doc asserting a durability guarantee nothing provides is worse than silence. WakeupFired carries no DelegateContext, and that exemption is named rather than wildcarded: a context is per-conversation state handed back on a reply, and a wakeup opens a conversation rather than continuing one. Carrying one would commit the host to persisting delegate context across arbitrary wall-clock time, which is #5467 Phase 3. The pin test asserts the WHOLE byte layout, not just the variant tag. Duration's encoding is a property of serde's impl, not of this crate, so a round-trip test would keep passing while every deployed delegate disagreed with the host about how long a week is. Verified: 127 tests pass; both clippy invocations clean; fmt clean. The byte pin was mutated twice and confirmed to FAIL -- once by swapping the field order, once by perturbing the fixture. Host half: freenet/freenet-core#3972. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV * feat(delegate): scheduled wakeup — host function out, WakeupFired in M1 from the wire review, and it changes the shape rather than the wording. `ScheduleWakeup` is no longer an OutboundDelegateMsg variant; it is `DelegateCtx::schedule_wakeup`, a host function in the `freenet_delegate_management` namespace. `WakeupFired` stays an inbound wire variant at tag 9. The reason is the batch boundary. A delegate's outbound messages are serialized as ONE batch (`delegate_interface.rs`, a single `bincode::serialize` of `Result<Vec<OutboundDelegateMsg>, _>`) and decoded whole by the host, so an outbound variant the host does not know fails the ENTIRE batch. A delegate built against this release returning `[ApplicationMessage(reply), ScheduleWakeup{..}]` to a current-release node would have the reply discarded along with the wakeup, and the user's action would silently do nothing. That is the direction ordinary rollout produces every time, since stdlib ships before core by policy. A host function fails the other way: an unimported function fails at instantiation with a named missing import -- loudly, once, at load. outbound variant would have put two opposite answers to one question in one release. WakeupFired stays a wire variant because that direction has no equivalent hazard: a delegate that cannot schedule never receives one. OUTBOUND_VARIANT_COUNT returns to 9; inbound stays at 10. Also from the review: - M4: `tag` is capped at MAX_WAKEUP_TAG_BYTES (128), enforced before the host call. Only the pending COUNT was bounded, which makes the tag an unbounded byte channel behind a bounded-looking limit -- the same defect as a cache capped by entry count while holding caller-chosen values. - M5: MIN_WAKEUP_DELAY (1s), documented as a host clamp. A delegate re-arming inside its own handler with a zero delay would otherwise spin the node in a tight wake loop; moving from a deadline to a delay did not remove that hazard, it only removed the "deadline in the past" spelling of it. - M6: documented what the context cache holds during a wakeup. It is keyed PER DELEGATE, not per conversation, and prunes after 10 minutes, so any wakeup worth scheduling outlives the context that existed when it was scheduled -- and a live context inside that window belongs to a different exchange. That is a second, independent reason WakeupFired carries no context: there is no coherent value to put there. Verified: 130 tests pass; both clippy invocations clean; wasm32 build clean; fmt clean. The two new bounds guards were mutated and confirmed to FAIL -- once by removing the tag cap, once by an off-by-one turning `>` into `>=`, which the exactly-at-the-cap test catches. Host half: freenet/freenet-core#3972, which must supply the delay measured from receipt, the MIN_WAKEUP_DELAY clamp, a bound on pending wakeups per delegate, and persistence across restart. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV * fix(review): enforce the delay floor, and stop calling guest checks bounds Round-2 findings. M1 is behavioural; the rest are the same defect in prose, which is that this crate was describing obligations on an unwritten host in the present tense. M1: `schedule_wakeup` now clamps `after` up to MIN_WAKEUP_DELAY itself. The doc said the host "will honour" the floor, that the constant "states the contract so a delegate author can rely on it", and that "#3972 enforces it" -- three lines above a paragraph correctly saying no host does anything yet. Two registers in one comment, and the second was the honest one. One line in the wrapper makes the sentence true of the API the delegate actually calls, and it is how MAX_WAKEUP_TAG_BYTES was already handled; enforcing one documented bound locally while delegating the other to an unwritten host had no stated reason. The clamp is a pure `clamp_wakeup_delay` rather than an inline `.max()`, because the call site is inside `cfg(target_family = "wasm")` and CI never executes that branch -- the wasm32 jobs build and lint only. A clamp living only there would be type-checked and unverified, which is the "twelve tests that never ran" shape #98 found in this same crate. M3: the test comment said the tag cap "must be enforced before the host call, not by the host". That is backwards, and it was addressed to the person who has to write the host check. `__frnt__delegate__schedule_wakeup` is an ordinary WASM import, so a delegate can declare its own extern block and pass a 10 MB tag without ever touching DelegateCtx. A guest-side check on a guest-controlled import is fail-fast convenience and can never be a bound. Only the host can enforce it. Generalised in the comment, since it is true of every bound this crate documents. M2: "the host bounds how many wakeups a delegate may hold pending" was present tense in two places and false. It is load-bearing rather than incidental -- the reader is told a count cap exists in order to explain why a size cap is also needed -- so it is now an obligation on #3972 alongside durability. M4/L1: deleted a stale CHANGELOG run. It claimed `Duration` was on the wire with a byte layout pinned by a test; neither survived the M1 conversion, which moved the delay to `after_millis: i64` across the FFI and removed the outbound pin entirely. It also carried a duplicate durability section and a `### Why a delay` heading that escaped the entry it belonged to. Replaced with a Compatibility section describing what actually ships. L2: removed the superseded no-context rationale from the `get_context` comment. Two rationales coexisted and the weaker one was what a maintainer hits first when editing the accessor. Verified: 133 tests pass; both clippy invocations clean; fmt clean. The clamp guard was mutated to an identity and confirmed to FAIL. Turning `<` into `<=` is an equivalent mutant -- identical output for every input -- so it is not evidence of a gap either way. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV * feat(delegate): scheduled wakeup — host function out, WakeupFired in `ScheduleWakeup` is a host function, `DelegateCtx::schedule_wakeup`, in the `freenet_delegate_management` namespace. `WakeupFired` is an inbound wire variant at tag 9. A delegate's outbound messages are serialized as ONE batch and decoded whole by the host, so an outbound variant the host does not know fails the ENTIRE batch: a delegate built against this release returning `[ApplicationMessage(reply), ScheduleWakeup{..}]` to a current-release node would have the reply discarded along with the wakeup. That is the direction ordinary rollout produces every time, since stdlib ships before core. A host function fails at instantiation instead, with a named missing import -- loudly, once, at load. ## Rebase onto main after #128 merged `rust/src/delegate_host.rs` conflicted; `CHANGELOG.md` merged cleanly. WHAT I KEPT: both sides, in full, at both conflict sites. #128 added the `SubscribeOutcome` material and this PR adds the wakeup material; they were inserted at the same two anchors (after the MAX_SUBSCRIPTION_LIST_BYTES assert, and at the end of the test modules) and are independent. Neither side was dropped or edited during the resolution -- the later commits on this branch apply their own review fixes on top, so editing the text at resolution time would have conflicted with them. CHECKED FOR THE SEAM: both PRs document what the host does and does not guarantee, which is where a present-tense claim could reappear. It did not. #128's side reads "a statement about *now*, not a durability promise" and "on freenet-core as it stands (pre-#4669)"; this side reads "expected to bound", "must clamp too, and does not yet", and "an obligation on freenet-core#3972, not a property of anything shipped". Grep for the old present-tense forms ("the host bounds", "host will honour", "#3972 enforces") returns nothing. The two blocks agree. ONE THING THE RESOLUTION BROKE, caught by the compiler: taking both sides verbatim at the test-module conflict dropped `subscribe_outcome_tests`'s closing brace and the following `#[cfg(test)]`, because the closing brace sat in shared trailing context rather than inside either side. Restored. Confirmed both modules actually execute rather than merely compiling: subscribe_outcome_tests 6 tests, schedule_wakeup_guard_tests 7, wakeup_delay_clamp_tests 3. Verified: 142 tests pass; both clippy invocations clean; wasm32 build clean; fmt clean. Host half: freenet/freenet-core#3972, which must supply the delay measured from receipt, the MIN_WAKEUP_DELAY clamp, a bound on pending wakeups per delegate, persistence across restart, and the opt-in property that keeps WakeupFired safe. Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV

…dispatchers The choice of an explicit error over a silent drop for `OutboundDelegateMsg::UnsubscribeContractRequest` was the most load-bearing decision in this PR and nothing exercised it. Every other hunk touching the variant just widens a pre-existing "unexpected variant -> panic" arm, which is compile-driven churn, not coverage. Two tests, one per dispatcher, because they fail through different machinery and a regression in either alone would be invisible: - `unsubscribe_contract_request_fails_the_run_rather_than_being_dropped` drives `Runtime::process_outbound` directly via the existing `bare_runtime()` fixture and asserts `Err`, that the message names the request and #5600, and that `results` stayed empty so nothing was forwarded as though handled. Both `processed` states are covered. - `delegate_unsubscribe_request_surfaces_not_implemented_to_the_client` scripts the mock runtime to emit the request and asserts the CLIENT sees `DelegateResponse(Err(_))` naming the same. The mock does not go through `process_outbound`, so this is the only cover for the executor loop's arm. Both assert the error TEXT, not merely `is_err()`: a delegate request against that handler can fail for unrelated reasons, so an `is_err()` check would pass just as happily with the arm deleted. Both were verified by making them FAIL. Replacing each arm with a silent drop turns the loop test's result into `Ok([])` -- an empty SUCCESS, which is exactly the failure being guarded against: it reads as an unsubscribe that happened while the subscription is still live, and leaves the delegate waiting forever for a response nobody will send. Refs #5600, freenet/freenet-stdlib#98 Claude-Session: https://claude.ai/code/session_01RbvrRHmbnmNWywtSgG8qEV

sanity mentioned this pull request Aug 30, 2026

ci: clippy never enables the contract feature, so the contract-side API has never been linted #100

Open

sanity force-pushed the feat/delegate-unsubscribe-introspection branch from fecdf1c to eea3c0a Compare August 30, 2026 20:18

sanity mentioned this pull request Aug 30, 2026

feat: scheduled wakeup primitive for delegates (ScheduleWakeup / WakeupFired) #82

Merged

sanity commented Aug 30, 2026

View reviewed changes

sanity changed the title ~~feat(delegate): subscription introspection, and pin every wire tag~~ feat(delegate): unsubscribe, subscription introspection, and pin every wire tag Aug 30, 2026

sanity mentioned this pull request Sep 7, 2026

feat(delegate): report whether a subscribe actually pinned the contract #128

Merged

sanity added 14 commits September 7, 2026 09:16

chore(wire): allow dead_code on the pre-#52 ContractState mirror

4615278

Its fields are decoded into and never read, which is the test: the decode either fails or produces something wrong. CI denies warnings. Claude-Session: https://claude.ai/code/session_014tq59dRUCNsHR1GuUkguHw

sanity force-pushed the feat/delegate-unsubscribe-introspection branch from d4ad47e to 46bdd33 Compare September 7, 2026 14:18

sanity commented Sep 7, 2026

View reviewed changes

sanity mentioned this pull request Sep 7, 2026

docs: record what is actually safe to change on the delegate wire #129

Merged

sanity merged commit bbc31fb into main Sep 7, 2026
11 checks passed

sanity deleted the feat/delegate-unsubscribe-introspection branch September 7, 2026 20:46

sanity mentioned this pull request Sep 9, 2026

build: bump freenet-stdlib to 0.10.0 freenet/freenet-core#5610

Merged

Navigation Menu

feat(delegate): unsubscribe, subscription introspection, and pin every wire tag - #98

feat(delegate): unsubscribe, subscription introspection, and pin every wire tag#98
sanity merged 15 commits into
mainfrom
feat/delegate-unsubscribe-introspection

sanity commented Aug 30, 2026 •

edited

Loading

Uh oh!

sanity left a comment

Uh oh!

sanity left a comment

Uh oh!

Uh oh!

sanity commented Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

sanity commented Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Approach

Compatibility

Testing

Also in this PR: two fixes in memory/buf.rs

Verification

Note for whoever rebases #82

The pins were verified to run, and to fail

A note on the external review

The host half: owner, and why it cannot be linked yet

Two findings from review that are the same lesson from opposite directions

Uh oh!

sanity left a comment

Choose a reason for hiding this comment

Comprehensive PR Review: #98

Summary

Code-First Analysis

Testing Assessment

Skeptical Findings

Big Picture Assessment

External Model (codex)

Documentation

Recommendations

Must Fix (Blocking)

Should Fix (Important)

Consider (Suggestions)

Verdict

Uh oh!

sanity left a comment

Choose a reason for hiding this comment

Re-review after rebase — Full tier

The rebase

Findings

On finding (4), because dismissing a High needs evidence

What the lenses verified rather than accepted

Guards were made to fail

External model

Non-blocking follow-ups

Verdict

Uh oh!

Uh oh!

sanity commented Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

sanity commented Aug 30, 2026 •

edited

Loading

Also in this PR: two fixes in `memory/buf.rs`